[codex] fix Synapse v1 follow-up review findings#19
Conversation
|
@codex review |
|
@copilot review |
|
@coderabbitai review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae22f5ba3e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| requestedCopies: 0, | ||
| complete: true, |
There was a problem hiding this comment.
Keep reset upload mock in sync
Because resetCommandMocks() is called at module load and before each test, the new successful-upload defaults here are overwritten by the reset implementation below, which still returns no requestedCopies or complete. Any test that relies on the shared upload mock after reset will now exercise undefined for those fields and report partially_uploaded, so update the reset mock to include the same fields.
Useful? React with 👍 / 👎.
Summary
Follow-up to the Synapse SDK v1 upgrade in #17. This PR fixes issues found in a deeper review of the v1 command mappings.
Findings Fixed
wallet summarysomonthlyAccountRateis backed by Synapse v1'slockupRatePerMonthinstead of the reserve amounttotalRateBasedLockup.rateBasedLockupoutput field.UploadResultviarequestedCopies,complete, andstatus: partially_uploadedwhen Synapse reports fewer committed copies than requested.FailedAttempt.errordirectly for upload copy failures, avoiding[object Object]diagnostics when Synapse returns structured failed-attempt records.Root Cause
The v1 upgrade adapted compile-time API changes but left two semantic mismatches: a lockup balance was still labeled as a monthly rate, and the high-level upload command discarded Synapse v1's explicit partial-completion signal.
Mergeability Evidence
cli/src/commands/upload.ts,cli/src/commands/wallet/summary.ts, and focused command tests.ae22f5ba3e02e278d743c3638d751e96e6cd6688.CLIpassed on the latest head.@codex review,@copilot review, and@coderabbitai reviewwere requested after the PR was mature; no review comments or unresolved threads were returned before merge.Validation
bun testbunx tsc --noEmitbunx biome check src testsbun run buildgit diff --check